Complete threads #332
Complete threads #332
Conversation
There was a problem hiding this comment.
I would be glad to get feddback on the validity of thread pointers validity after thread free.
I guess this widens the door to use after free issues and segfaults (already opened with other thread methods), but not too sure.
AFAICT, this is fine. Thread instances are garbage collected like any other value. If an Oz variable holds a reference to a thread, and that variable is alive, then the Thread instance stays alive.
You might want to check that you don't try to resume/suspend a terminated Thread, though. IIUC currently those are C++ assertions, but if coming from explicit calls in Oz, which should throw proper Oz exceptions instead.
| static void call(VM vm, In thread) { | ||
| Runnable* runnable = getArgument<Runnable*>(vm, thread); | ||
|
|
||
| runnable->suspend(); |
There was a problem hiding this comment.
This won't be enough if a thread wants to suspend itself. The easiest way to deal with that might be to do the dispatch in Oz, with:
suspend: proc {$ T}
if {Boot_Thread.this} == T then
{Boot_Thread.preempt}
else
{Boot_Thread.suspend T}
end
endThere was a problem hiding this comment.
Ah, indeed. I do not know if suspending itself should preempt or not. I guess it makes sense, but we still have to suspend it. Suspend is like a preempt until resume is called.
There was a problem hiding this comment.
Hum, you're right, it should definitely suspend, not just preempt. But if we only suspend, I think the emulator loop won't notice that it needs to unschedule itself now. It's a bit tricky. Maybe the following works:
suspend: proc {$ T}
{Boot_Thread.suspend T}
if {Boot_Thread.this} == T then
{Boot_Thread.preempt}
end
endbut maybe that causes all sorts of complications (as some bytecode instructions will keep running even though the thread is marked as suspended.
It might not be possible to do this properly in Oz after all.
There was a problem hiding this comment.
A thread is never "marked" as suspended. It is just removed from the active queues. suspend + preempt should be perfectly fine.
AFAIK the Thread value in oz wraps a raw pointer to the vm thread structure. I do not think these are reference counted. |
|
Oh, and of course I cannot test this PR because of #330 |
|
Oz has a precise semantics :-). Why does this take a thread as argument ?
|
No idea 😕 |
It does wrap a pointer to the vm |
|
Good to know ! Thanks for the detailed explanation :-) |
A draft implementation of thread methods still missing.
I would be glad to get feddback on the validity of thread pointers validity after thread free.
I guess this widens the door to use after free issues and segfaults (already opened with other thread methods), but not too sure.
/cc @sjrd
Based on top of #331
Fixes mozart/mozart#200